Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

move aliases of highway=track from terms to aliases field #470

Merged
merged 8 commits into from
Sep 29, 2022

Conversation

westnordost
Copy link
Contributor

@westnordost westnordost commented May 26, 2022

I would also have renamed the preset to Land-Access Road and moved Track Road to the aliases because with aliases, there is no need anymore to have any names with a / but since you just implemented interpreting the aliases field in iD, I guess you are aware of this possibility and had your reasons to keep it that way.

See #288 (comment)

"unmaintained",
"woods road"
"unmaintained road",
"dirt road"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to keep terms sorted alphabetically. 🤓

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize they were before. So, should I change this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, please

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iD ranks terms by their position in the list, so sorting them alphabetically promotes some rarer terms over more common terms. This might be less of a problem now that some terms are aliases, but it’s hard to test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? Is this documented somewhere (+documented for translators)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, either I’m mistaken or it’s no longer in place. It looks like nowadays iD only considers the index of a match against the preset name but only filters by terms.

@tyrasd tyrasd added this to the v3.4 milestone May 26, 2022
@tyrasd tyrasd added the new-label changes the name, aliases or terms of a preset label May 26, 2022
@westnordost westnordost requested a review from tyrasd May 26, 2022 15:11
@tyrasd
Copy link
Member

tyrasd commented May 26, 2022

I guess whether it should be an alias depends how exactly "alias" is defined and how it is (going) to be shown in the UI.

I'm not really sure what the difference is between "aliases" and "terms"

Well, the problem here is that this property was never documented (see ideditor/schema-builder#28), nobody really knows.

The most conservative approach would be to use aliases only for proper synonyms (here: land access road and track road). Whether it makes sense to also include names which describe a strict subset of features represented by the preset can be discussed IMO (here for example: forest track, agricultural road, etc.). I would abstain from including terms as aliases which can cover features not represented by the preset (here: dirt road, unmaintained road).

@westnordost
Copy link
Contributor Author

Yeah... I will answer in ideditor/schema-builder#28

@westnordost westnordost marked this pull request as draft May 26, 2022 19:58
data/presets/highway/track.json Outdated Show resolved Hide resolved
data/presets/highway/track.json Outdated Show resolved Hide resolved
data/presets/highway/track.json Show resolved Hide resolved
@westnordost
Copy link
Contributor Author

(FYI I consider this blocked until ideditor/schema-builder#28 came to a conclusion)

@tyrasd
Copy link
Member

tyrasd commented May 30, 2022

(FYI I consider this blocked until ideditor/schema-builder#28 came to a conclusion)

see ideditor/schema-builder#28 (comment) – please only keep Track Road and Land Access Road in the list of aliases and move the rest back to the terms.

- add suggestions from @1ec5
- demote all except land-access road and track road to terms
@westnordost westnordost marked this pull request as ready for review September 29, 2022 12:24
@westnordost
Copy link
Contributor Author

Applied suggestions and #470 , ready for merge

@tyrasd tyrasd merged commit 49ec943 into openstreetmap:main Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-label changes the name, aliases or terms of a preset
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants